-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Supports public and private git repos when using shorted repo syntax #2992
Conversation
@bestander any thoughts on the approach taken here? |
so far looks reasonable, go on :) |
Cheers will finish this up hopefully in the week. |
const fragment: ExplodedFragment = { | ||
user: 'foo', | ||
repo: 'bar', | ||
hash: '', | ||
}; | ||
|
||
const expected = 'git+ssh://[email protected]/' + fragment.user + '/' + fragment.repo + '.git'; | ||
expect(GitHubResolver.getGitSSHUrl(fragment)).toBe(expected); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously the getGitHTTPUrl
test was actaully running getGitSSHUrl
instead!
const fragment: ExplodedFragment = { | ||
user: 'foo', | ||
repo: 'bar', | ||
hash: '', | ||
}; | ||
|
||
const expected = 'git+ssh://[email protected]/' + fragment.user + '/' + fragment.repo + '.git'; | ||
expect(GitLabResolver.getGitSSHUrl(fragment)).toBe(expected); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously the getGitHTTPUrl
test was actaully running getGitSSHUrl
instead!
CI seems to be failing on random tests unrelated to my changes, this work is now ready for someone to review. |
Awesome work, @thedumbterminal! |
Just tried it and it doesn't work (github repo shorthand with private repo); it prompted me to enter my ssh key's passphrase but I cannot actually enter anything at all. |
@gsklee You need to make sure you have your ssh key setup in your terminal first, if you are using linux or mac osx then you can run:
Then try running the yarn command again. |
Summary
When using the shortened git syntax such as
github:thedumbterminal/grunt-gemnasium#vx
, yarn tests for HTTP access using a generated URL ending in.git
this will always return a301
status even if the user does not have permission to access the repo (private repos would require auth first).This change now uses a full git URL which does not end in
.git
this means that private repos will fail with404
so then SSH auth is attempted instead, as long as the correct SSH keys are setup in the users terminal access to the private repo will succeed.Addresses existing issue: #2774
Test plan
Added new unit tests for the new
getGitHTTPBaseUrl
method.This can be manually tested in the following way:
mkdir /tmp/example; cd /tmp/example; npm init -y
"grunt-gemnasium": "github:thedumbterminal/grunt-gemnasium#v1.1.1"
yarn --verbose
, both modules should be installed.